Skip to content

Fix type merging for unknown effects#8833

Merged
stevenfontanella merged 1 commit into
mainfrom
fix-effects-merge
Jun 11, 2026
Merged

Fix type merging for unknown effects#8833
stevenfontanella merged 1 commit into
mainfrom
fix-effects-merge

Conversation

@stevenfontanella

Copy link
Copy Markdown
Member

Fixes #8831. See the incorrect optimization described in that issue.

@stevenfontanella
stevenfontanella marked this pull request as ready for review June 11, 2026 21:18
@stevenfontanella
stevenfontanella requested a review from a team as a code owner June 11, 2026 21:18
@stevenfontanella
stevenfontanella requested review from kripken and removed request for a team June 11, 2026 21:18
@stevenfontanella
stevenfontanella merged commit 193792f into main Jun 11, 2026
16 checks passed
@stevenfontanella
stevenfontanella deleted the fix-effects-merge branch June 11, 2026 21:52
stevenfontanella added a commit that referenced this pull request Jul 22, 2026
Part of #8615. Fixes #8833 which was wrong. The earlier fix never
touched `newTypeEffects` at all. It passed tests and fixed the breakage
only incidentally by being maximally conservative and losing all
indirect call effects in the case of a type update. Fix this logic and
add unit tests to vet this code better.

Drive-by fixes:
* Clear indirectCallEffects during --discard-global-effects, before
recomputing global effects, and if a pass's `addsEffects()` is true.
Clearing it when recomputing global effects is necessary to remove stale
entries for types that no longer exist after type rewriting (although it
should make no difference to optimizations).
* Fix use-after-free in tests due to static storage of HeapTypes / Types
+ destroyAllTypesForTestingPurposesOnly. The static HeapType / Type in
`getMutI8Array` and similar functions referred to an index in
`globalHeapTypeStore` / `globalTupleStore`, which gets cleared in unit
tests with `destroyAllTypesForTestingPurposesOnly()`, causing the index
to refer to garbage. Change these to not use static storage so that
they're always in sync with `globalHeapTypeStore` / `globalTupleStore`.
* More changes to support gmock. #8879 partially addressed this but I
still hit some linker errors when adding new test code using gmock. The
issue is that the earlier PR didn't change anything when BUILD_FUZZTEST
is false.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect optimization/effects with GlobalEffects in closed-world

2 participants